refactor(Android, FormSheet v5): Introduce invalidation flags for updates#4262
refactor(Android, FormSheet v5): Introduce invalidation flags for updates#4262t0maboro wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Android FormSheetDialogManager update flow to mirror the iOS “diff + invalidate + flush” architecture, introducing explicit invalidation flags and deferring all work into a single coordinated flushPendingUpdates() pass.
Changes:
- Added
FormSheetInvalidationFlagsto track presentation/appearance/behavior invalidations. - Refactored
applyConfigto compute invalidations instead of performing immediate side effects. - Centralized update execution into
flushPendingUpdates()with dedicated update helpers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| android/src/main/java/com/swmansion/rnscreens/gamma/modals/formsheet/FormSheetInvalidationFlags.kt | Adds a new invalidation-flags holder for coordinating updates. |
| android/src/main/java/com/swmansion/rnscreens/gamma/modals/formsheet/FormSheetDialogManager.kt | Switches from immediate updates to invalidation + flush-based update execution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
t0maboro
force-pushed
the
@t0maboro/formsheet-presentation-manager
branch
from
b808770 to
a748157
Compare
t0maboro
changed the title
t0maboro
force-pushed
the
@t0maboro/formsheet-android-invalidation-flags
branch
from
4e8a215 to
719193e
Compare
t0maboro
changed the title
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
t0maboro
force-pushed
the
@t0maboro/formsheet-android-invalidation-flags
branch
from
719193e to
94c119e
Compare
| } | ||
|
|
||
| private fun updateSheetAppearance() { | ||
| container.setGrabberVisible(formSheetConfig.prefersGrabberVisible) |
There was a problem hiding this comment.
note: a dedicated AppearanceCoordinator will be introduced later, once I add support for e.g. corner radius
kmichalikk
left a comment
kmichalikk
left a comment
There was a problem hiding this comment.
Something's off. On basic functionality test, origin/main starts with detent 0.3 and here it's like 2/3 and it doesn't respond to changes on hot reload whereas on main it does.
Grabber visibility works fine.
|
I'm also wondering if this change here is not a bit redundant since we're not calling this anywhere else and just have flags set which is immediately followed by flush and this is the only one code path. Maybe this will grow later, then it does make sense. |
Description
Following the iOS architecture (which has distinct Presentation, Appearance, and Behavior providers along with update flags), this PR introduces an
InvalidationFlagsforFormSheetDialogManageronAndroid.Instead of applying configuration changes when props change, the
applyConfigmethod now evaluates what changed, raises the appropriate flags, and defers the actual execution to a coordinatedflushPendingUpdatespass.Note: as for iOS, reopening the sheet automatically invalidates both
AppearanceandBehavior. This ensures the nativeBottomSheetBehaviorand layout are strictly synchronized with the React Native state, preventing stale configurations on re-mounts.Changes
FormSheetInvalidationFlagsflushPendingUpdatesBefore & after - visual documentation
N/A - refactor
Test plan
N/A - refactor
Checklist